Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Record XrSim tests for all the sample projects #170

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Record XrSim tests for all the sample projects #170

merged 1 commit into from
Jul 17, 2024

Conversation

dsnopek
Copy link
Collaborator

@dsnopek dsnopek commented Jun 19, 2024 •
edited
Loading

Now that we have tests via the Meta XR Simulator, this PR is adding tests for the two sample projects that are currently in the repo.

Some notes:

  • Cylinder composition layers don't work quite right in the simulator - it's like the faces are wound in reverse, such that it gets back face culled when looking in the correct direction (but the layer is visible when looking in the opposite direction)
  • Only basic passthrough is supported by the simulator - geometry passthrough and all the styles don't work
  • I had to downgrade to v64 of the simulator because passthrough wasn't working UPDATE: These issues are fixed in v66, but it has it's own issues. I'm just going to use v65, even though passthrough isn't quite right - when there is a fully fixed version, we can re-record the VRS file.
  • Since we have to update the simulator version in the CI manually, we'll be able to update the VRS files if/when the above bugs are fixed (and the current VRS files can help us notice when those bugs are fixed, since the tests will fail)
  • UPDATE: I switched to using Git LFS to store the VRS files per the discussion below

This is a draft since I don't think the CI is going to work on the first try because I reorganized some things. :-) Once it's working, I'll take it out of draft. (Also, I'm temporarily basing this on PR #149 to make iteration faster.)

@dsnopek dsnopek added the enhancement New feature or request label Jun 19, 2024
@dsnopek dsnopek added this to the 3.0.0 milestone Jun 19, 2024
@dsnopek dsnopek marked this pull request as draft June 19, 2024 21:00
@dsnopek dsnopek force-pushed the xrsim-sample-tests branch 2 times, most recently from d75d8e5 to c1c59b3 Compare June 20, 2024 14:27
@dsnopek
Copy link
Collaborator Author

dsnopek commented Jun 20, 2024

Question: Should we use Git LFS for the *.vrs files?

They don't get that big, but if you make one that takes more than ~2 screenshots, they can get big-ish. The one for the passthrough sample on this PR is 57mb (I think I took 5 screenshots?). If we make a couple of revisions to that, it'll start to add up in the overall repo size when doing a full clone. The other *.vrs files are smaller: 5.9mb and 16mb (I only took 1 or 2 screenshots)

I'm personally leaning towards using Git LFS, but since we aren't using it at all in this repo yet, I wanted to run it by other folks first!

/cc @m4gr3d @devloglogan @BastiaanOlij

@dsnopek dsnopek force-pushed the xrsim-sample-tests branch 14 times, most recently from c3365e4 to 5524422 Compare June 24, 2024 21:46
@devloglogan
Copy link
Collaborator

Seeing that any .vrs files we add potentially need to be updated if the related sample/feature is changed, and that "more than ~2 screenshots" feels like an extremely low bar to hit for having a large file, I'd agree that git LFS seems like the right choice.

@dsnopek dsnopek force-pushed the xrsim-sample-tests branch 3 times, most recently from 070263f to 0a7a922 Compare June 25, 2024 20:54
@dsnopek dsnopek force-pushed the xrsim-sample-tests branch 5 times, most recently from 7246ccc to 34841b3 Compare July 9, 2024 16:43
@dsnopek dsnopek changed the title [DRAFT] Record XrSim tests for the composition layer and passthrough sample projects [DRAFT] Record XrSim tests for all the sample projects Jul 9, 2024
@dsnopek dsnopek force-pushed the xrsim-sample-tests branch 8 times, most recently from 8ed05cd to 00cbc4a Compare July 9, 2024 20:23
@dsnopek dsnopek changed the title [DRAFT] Record XrSim tests for all the sample projects Record XrSim tests for all the sample projects Jul 9, 2024
@dsnopek dsnopek marked this pull request as ready for review July 9, 2024 20:45
@dsnopek
Copy link
Collaborator Author

dsnopek commented Jul 9, 2024

Finally taking this one out of draft! I've got simple tests recorded for all the sample projects, except for the Scene API one: it works great when running it manually, but has some issue when running automatically - I suspect an XR Simulator bug, but I'm not really sure. In any case, it's something we can solve in a follow-up, rather than delaying on getting all the rest in.

This PR also includes some good refactoring and improvements of the CI for running these tests.

And I did end up moving the VRS files into Git LFS.

This was referenced Jul 10, 2024
Merged
Merged
devloglogan
devloglogan approved these changes Jul 10, 2024
View reviewed changes
Copy link
Collaborator

@devloglogan devloglogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@devloglogan devloglogan mentioned this pull request Jul 11, 2024
Merged
@dsnopek dsnopek mentioned this pull request Jul 11, 2024
Merged
@m4gr3d m4gr3d merged commit a3eef11 into GodotVR:master Jul 17, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m4gr3d m4gr3d m4gr3d approved these changes

@devloglogan devloglogan devloglogan approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@dsnopek @devloglogan @m4gr3d